feat(jellyfish-api-core): Add updatemasternode RPC typings #1847

marktanrj · 2022-11-01T04:47:45Z

What this PR does / why we need it:

Add RPC updatemasternode

Which issue(s) does this PR fixes?:

Fixes part of #1842

netlify · 2022-11-15T06:44:49Z

✅ Deploy Preview for jellyfishsdk ready!

Name	Link
🔨 Latest commit	`b995da1`
🔍 Latest deploy log	https://app.netlify.com/sites/jellyfishsdk/deploys/6373352d0f8fa100089852b4
😎 Deploy Preview	https://deploy-preview-1847--jellyfishsdk.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

codeclimate · 2022-11-15T06:44:54Z

Code Climate has analyzed commit 399b613 and detected 0 issues on this pull request.

View more on Code Climate.

netlify · 2022-11-15T06:45:35Z

✅ Deploy Preview for jellyfishsdk ready!

Name	Link
🔨 Latest commit	`399b613`
🔍 Latest deploy log	https://app.netlify.com/sites/jellyfishsdk/deploys/638595c7f2e7090008075e72
😎 Deploy Preview	https://deploy-preview-1847--jellyfishsdk.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

codecov · 2022-11-15T06:53:43Z

Codecov Report

Base: 90.81% // Head: 92.12% // Increases project coverage by +1.31% 🎉

Coverage data is based on head (399b613) compared to base (a6e96d4).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1847      +/-   ##
==========================================
+ Coverage   90.81%   92.12%   +1.31%     
==========================================
  Files         357      361       +4     
  Lines       10430    10477      +47     
  Branches     1316     1320       +4     
==========================================
+ Hits         9472     9652     +180     
+ Misses        912      786     -126     
+ Partials       46       39       -7

Impacted Files	Coverage Δ
...ages/jellyfish-api-core/src/category/masternode.ts	`96.00% <100.00%> (+16.83%)`	⬆️
...atus-api/src/controllers/OracleStatusController.ts	`19.56% <0.00%> (-78.27%)`	⬇️
...s/playground-api-client/src/PlaygroundRpcClient.ts	`33.33% <0.00%> (-66.67%)`	⬇️
...ckages/jellyfish-network/src/TransactionSkipped.ts	`37.50% <0.00%> (-50.00%)`	⬇️
apps/libs/blockchaincpp/src/BlockchainCppModule.ts	`50.00% <0.00%> (-38.24%)`	⬇️
packages/jellyfish-wallet-mnemonic/src/hd_node.ts	`67.79% <0.00%> (-28.82%)`	⬇️
packages/whale-api-client/src/api/masternodes.ts	`71.42% <0.00%> (-28.58%)`	⬇️
...transaction-builder/src/txn/txn_builder_account.ts	`67.56% <0.00%> (-27.03%)`	⬇️
...jellyfish-transaction/src/script/dftx/dftx_misc.ts	`75.00% <0.00%> (-25.00%)`	⬇️
packages/whale-api-client/src/api/address.ts	`77.77% <0.00%> (-22.23%)`	⬇️
... and 66 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

github-actions · 2022-11-15T07:00:05Z

Docker build preview for jellyfish/apps is ready!

Built with commit 428f618

ghcr.io/jellyfishsdk/legacy-api:pr-1847
ghcr.io/jellyfishsdk/ocean-api:pr-1847
ghcr.io/jellyfishsdk/playground-api:pr-1847
ghcr.io/jellyfishsdk/status-api:pr-1847
ghcr.io/jellyfishsdk/whale-api:pr-1847

You can also get an immutable image with the commit hash

ghcr.io/jellyfishsdk/legacy-api:428f618e45153512d51f01070f4c011cb320ad8a
ghcr.io/jellyfishsdk/ocean-api:428f618e45153512d51f01070f4c011cb320ad8a
ghcr.io/jellyfishsdk/playground-api:428f618e45153512d51f01070f4c011cb320ad8a
ghcr.io/jellyfishsdk/status-api:428f618e45153512d51f01070f4c011cb320ad8a
ghcr.io/jellyfishsdk/whale-api:428f618e45153512d51f01070f4c011cb320ad8a

shohamc1

@imkven

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts

imkven · 2022-11-27T05:09:50Z

I have referred to /test/functional/rpc_updatemasternode.py to write tests in updatemasternode.test.ts but I have 2 tests that do not get the error message in Python. Can you help check? @shohamc1 CC @marktanrj @infinia-yzl

○ skipped Test owner update without collateral input
○ skipped Node 1 try to update node 0 which should be rejected

shohamc1 · 2022-11-27T07:26:37Z

○ skipped Test owner update without collateral input

This test requires manually building an incorrect transaction, I think it is better suited for the tests on #1863

○ skipped Node 1 try to update node 0 which should be rejected

Fixed in ca6ca20, but we should refactor the tests in the PR to use the TestingGroup on all tests.

shohamc1

fuxingloh

LGTM, with 1 minor change for clarity

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts

infinia-yzl

State is fully deterministic.

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts

fuxingloh

LGTM, with a few suggestions that we don't have to apply now. More than 2000 test cases, I believe, are written in the "old style" that has been discouraged now.

fuxingloh · 2022-11-29T06:02:01Z

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts

+
+  it('should updateMasternode with utxos', async () => {
+    const balance = await container.call('getbalance')
+    expect(balance >= 2).toBeTruthy()


Kinda weird to write it like this, it should be

Suggested change

expect(balance >= 2).toBeTruthy()

expect(balance).toBeGreaterThanOrEqual(2)

fuxingloh · 2022-11-29T06:07:20Z

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts

+      expect(masternodes[masternodeId]).toBeTruthy()
+      expect(masternodes[masternodeId].state).toStrictEqual('TRANSFERRING')
+      expect(masternodes[masternodeId].ownerAuthAddress).toStrictEqual(initialAddress)


Suggested change

expect(masternodes[masternodeId]).toBeTruthy()

expect(masternodes[masternodeId].state).toStrictEqual('TRANSFERRING')

expect(masternodes[masternodeId].ownerAuthAddress).toStrictEqual(initialAddress)

expect(data).toStrictEqual(expect.objectContaining({

state: 'TRANSFERRING',

ownerAuthAddress: initialAddress

}))

For better readability, it's better to write tests like this. Most of the tests written previously were written the previous "style" that we've since discouraged. We don't have to change it now but just a thought for the future.

Add updateMasternode rpc (WIP)

b4df405

marktanrj requested review from fuxingloh, canonbrother, ivan-zynesis and chee-chyuan as code owners November 1, 2022 04:47

github-actions bot added apps/website area/packages labels Nov 1, 2022

marktanrj changed the title ~~Add RPC updatemasternode~~ feat(jellyfish-api-core): Add RPC updatemasternode Nov 1, 2022

github-actions bot added the kind/feature New feature request label Nov 1, 2022

marktanrj changed the title ~~feat(jellyfish-api-core): Add RPC updatemasternode~~ feat(jellyfish-api-core): Add updatemasternode RPC typings Nov 1, 2022

marktanrj marked this pull request as draft November 1, 2022 04:49

marktanrj removed request for fuxingloh, canonbrother, ivan-zynesis and chee-chyuan November 1, 2022 04:49

marktanrj added 2 commits November 2, 2022 14:43

Add test cases

80cee4a

Undo update config that was accidentally committed

cc0f828

marktanrj changed the base branch from marktanrj/updatemasternode to main November 14, 2022 03:03

marktanrj mentioned this pull request Nov 14, 2022

Support CustomTxType::UpdateMasternode on Jellyfish #1842

Closed

marktanrj and others added 2 commits November 15, 2022 14:42

Update testcases

b995da1

Merge branch 'main' into marktanrj/updatemasternode-rpc-typing

27721d8

marktanrj marked this pull request as ready for review November 15, 2022 06:44

marktanrj added 2 commits November 16, 2022 10:25

Update image version

bc30eb9

Update image version

bb06945

marktanrj requested a review from cwkang1998 as a code owner November 16, 2022 06:14

shohamc1 suggested changes Nov 25, 2022

View reviewed changes

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts Outdated Show resolved Hide resolved

imkven and others added 9 commits November 25, 2022 14:29

Test updating several MNs owners in the same block

7298063

Refactor: remove try-catch blocks

5cd77a7

Refactor: do not expect undefined

a475b3c

Update updatemasternode.test.ts

23c2528

Add test: Node 1 try to update node 0 which should be rejected

52eefa2

Update updatemasternode.test.ts

301b95d

Update updatemasternode.test.ts

2052b11

Update updatemasternode.test.ts

a235d1a

Update updatemasternode.test.ts

dfd70ae

Fix incorrect auth test

ca6ca20

imkven added 3 commits November 28, 2022 10:24

Merge branch 'main' into marktanrj/updatemasternode-rpc-typing

4f9ce0f

Update updatemasternode.test.ts

d4d3cb4

Merge branch 'main' into marktanrj/updatemasternode-rpc-typing

427bf4b

infinia-yzl previously approved these changes Nov 28, 2022

View reviewed changes

infinia-yzl requested a review from shohamc1 November 28, 2022 09:54

shohamc1 previously approved these changes Nov 28, 2022

View reviewed changes

fuxingloh suggested changes Nov 28, 2022

View reviewed changes

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts Outdated Show resolved Hide resolved

packages/jellyfish-api-core/__tests__/category/masternode/updatemasternode.test.ts Outdated Show resolved Hide resolved

Update updatemasternode.test.ts

f4620aa

imkven dismissed stale reviews from shohamc1 and infinia-yzl via f4620aa November 29, 2022 03:34

Merge branch 'main' into marktanrj/updatemasternode-rpc-typing

3c35b7e

infinia-yzl suggested changes Nov 29, 2022

View reviewed changes

Update updatemasternode.test.ts

399b613

fuxingloh approved these changes Nov 29, 2022

View reviewed changes

fuxingloh merged commit 68d604a into main Nov 29, 2022

fuxingloh deleted the marktanrj/updatemasternode-rpc-typing branch November 29, 2022 06:09

fuxingloh mentioned this pull request Jan 9, 2023

DeFi Blockchain RPC Implementations #48

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(jellyfish-api-core): Add updatemasternode RPC typings #1847

feat(jellyfish-api-core): Add updatemasternode RPC typings #1847

marktanrj commented Nov 1, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022

codeclimate bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

codecov bot commented Nov 15, 2022 •

edited

Loading

github-actions bot commented Nov 15, 2022 •

edited

Loading

shohamc1 left a comment

imkven commented Nov 27, 2022 •

edited

Loading

shohamc1 commented Nov 27, 2022

shohamc1 left a comment

fuxingloh left a comment

infinia-yzl left a comment

fuxingloh left a comment

fuxingloh Nov 29, 2022

fuxingloh Nov 29, 2022

	expect(balance >= 2).toBeTruthy()
	expect(balance).toBeGreaterThanOrEqual(2)

feat(jellyfish-api-core): Add updatemasternode RPC typings #1847

feat(jellyfish-api-core): Add updatemasternode RPC typings #1847

Conversation

marktanrj commented Nov 1, 2022 • edited Loading

What this PR does / why we need it:

Which issue(s) does this PR fixes?:

netlify bot commented Nov 15, 2022

✅ Deploy Preview for jellyfishsdk ready!

codeclimate bot commented Nov 15, 2022 • edited Loading

netlify bot commented Nov 15, 2022 • edited Loading

✅ Deploy Preview for jellyfishsdk ready!

codecov bot commented Nov 15, 2022 • edited Loading

Codecov Report

github-actions bot commented Nov 15, 2022 • edited Loading

shohamc1 left a comment

Choose a reason for hiding this comment

imkven commented Nov 27, 2022 • edited Loading

shohamc1 commented Nov 27, 2022

shohamc1 left a comment

Choose a reason for hiding this comment

fuxingloh left a comment

Choose a reason for hiding this comment

infinia-yzl left a comment

Choose a reason for hiding this comment

fuxingloh left a comment

Choose a reason for hiding this comment

fuxingloh Nov 29, 2022

Choose a reason for hiding this comment

fuxingloh Nov 29, 2022

Choose a reason for hiding this comment

marktanrj commented Nov 1, 2022 •

edited

Loading

codeclimate bot commented Nov 15, 2022 •

edited

Loading

netlify bot commented Nov 15, 2022 •

edited

Loading

codecov bot commented Nov 15, 2022 •

edited

Loading

github-actions bot commented Nov 15, 2022 •

edited

Loading

imkven commented Nov 27, 2022 •

edited

Loading